Skip to content

Add manual closing functionality#8

Closed
sunsided wants to merge 1 commit into
mainfrom
feature/manual-close
Closed

Add manual closing functionality#8
sunsided wants to merge 1 commit into
mainfrom
feature/manual-close

Conversation

@sunsided

@sunsided sunsided commented Mar 8, 2024

Copy link
Copy Markdown
Owner

This is tricky due to potential double frees.


Provides some initial work for #1, even though it still does not support asynchronous drops.

@sunsided sunsided self-assigned this Mar 8, 2024
@sunsided
sunsided force-pushed the main branch 2 times, most recently from 06b2c9e to 2414cdf Compare February 22, 2025 06:13
sunsided added a commit that referenced this pull request Jun 2, 2026
Modernizes `async-tempfile` for the 0.8.0 release. The crate now
compiles with `#![forbid(unsafe_code)]`, generates unpredictable
temporary names, and gains a builder plus `keep`/`persist` for turning a
temporary into a permanent file or directory. Existing APIs are
unchanged.

## Why

The drop machinery relied on `ManuallyDrop` + `unsafe`, automatically
generated names were predictable and created without `O_EXCL` (a
preexisting-file / symlink race), and there was no way to keep or
relocate a temporary. This release closes those gaps while keeping the
public surface backward compatible.

## Key changes

- **Forbid unsafe.** The local file handle is declared before the shared
core, so field drop order closes it before the core deletes the file
(still correct on Windows). The `ManuallyDrop`/`unsafe` machinery is
gone.
- **Lock-free ownership.** Ownership lives in an atomic so `Drop` can
read it without ever blocking, and `keep`/`persist`/`drop_async` can
flip it at runtime.
- **`keep` and `persist`.** Disable auto-deletion, or move the
file/directory to a permanent path. Both are cancellation- and
panic-safe: the synchronous `Drop` stays armed as a backstop and is
disarmed only after the operation succeeds.
- **Builder.** `TempFile::builder()` / `TempDir::builder()` configure
prefix, suffix and target directory.
- **Hardened names.** Auto-generated names are seeded from the OS RNG
(plus a per-process counter for guaranteed local uniqueness) and created
with an exclusive (`O_EXCL`) create. User-supplied names keep
create-or-open behavior.
- **Async drop.** `drop_async` deletes via `tokio::fs` directly when it
is the sole owner, instead of offloading a blocking synchronous drop.
- **Fewer descriptors.** Each `TempFile` previously held a redundant
keep-alive handle; removing it halves open descriptors per file.
- Edition 2024, MSRV 1.85, and a `Taskfile.dist.yaml` mirroring CI.

## Notes for reviewers

The drop-safety contract is the heart of the change, so start in
`src/tempfile.rs`: the `TempFile`/`TempFileCore` struct definitions
(note the field-order comment), then `Drop for TempFileCore`, `persist`,
and `drop_async` — these implement the "synchronous Drop is the
always-armed backstop; async paths disarm only after success" rule.
`src/tempdir.rs` mirrors it. `tests/drop_safety.rs` encodes that
contract: cleanup under cancellation and panic, a deadlock watchdog on
both runtime flavors, and a leak/uniqueness stress test (it passes under
`ulimit -n 64`). `src/builder.rs` and `tests/builder.rs` are
self-contained.

Behavior change to be aware of: the auto-name path now uses `O_EXCL`, so
it will not silently reuse an existing file the way the old `.create()`
path could; user-supplied names are unaffected.

Closes #11. Closes #12. Refs #1 (improves `drop_async`; true async drop
remains blocked on stable Rust). Supersedes the draft in #8, whose
double-free concern is removed by dropping `ManuallyDrop`.
@sunsided

sunsided commented Jun 2, 2026

Copy link
Copy Markdown
Owner Author

Resolved — manual closing is implemented

The manual-closing functionality this issue asked for now exists, and the double-free concern is handled structurally:

API (TempFile + TempDir):

Double-free prevention (the "tricky" part):

  • Clones share Arc<Core>; Core::drop runs exactly once, when the last reference drops — no cross-clone double delete.
  • drop_async / close gate on Arc::into_inner: only the sole owner deletes; otherwise cleanup falls to the remaining references' Drop.
  • AtomicOwnership is flipped to Borrowed only after a confirmed removal, so the synchronous Drop backstop becomes a no-op. FS deletion is idempotent (NotFound ignored), so even the error-retry path can't double-fault.

The remaining caveat from the original description — "still does not support asynchronous drops" — is the language-level AsyncDrop feature, tracked separately in #1 (blocked on unstable Rust). drop_async is the interim manual substitute.

Closing as completed.

@sunsided sunsided closed this Jun 2, 2026
sunsided added a commit that referenced this pull request Jun 2, 2026
Adds close(self) -> io::Result<()>, the synchronous error-observable sibling of drop_async. Same Arc::into_inner sole-owner gating and disarm-after-removal semantics; on a deletion error it disarms and returns Err, leaving the file/dir deterministically in place (no pointless Drop retry of the same syscall). Rounds out the manual-closing API (keep/persist/drop_async/close) from #8.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant